-
Notifications
You must be signed in to change notification settings - Fork 31
RHAI-ENG-306-modify-docs-on-deploying-llamastackdistribution-instanc… #898
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RHAI-ENG-306-modify-docs-on-deploying-llamastackdistribution-instanc… #898
Conversation
WalkthroughUpdated documentation: changed prerequisite wording for the Llama Stack Operator and revised LlamaStackDistribution guidance to use an internal image reference ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
modules/deploying-a-llama-model-with-kserve.adoc (1)
13-16
: Anchor/terminology mismatch between “activated” wording and linked sectionThe prerequisite now says activated the Llama Stack Operator, but the link that follows still points to
#installing-the-llama-stack-operator_rag
.
Consider renaming the target anchor (and page heading) to activating for consistency, or adjust the prose here back to installed to avoid sending readers to a differently-worded section.No functional impact, but improving this avoids cognitive friction.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
modules/deploying-a-llama-model-with-kserve.adoc
(1 hunks)modules/deploying-a-llamastackdistribution-instance.adoc
(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: bredamc
PR: opendatahub-io/opendatahub-documentation#860
File: modules/using-llm-as-a-judge-metrics-with-lmeval.adoc:62-65
Timestamp: 2025-07-16T09:05:50.422Z
Learning: In the opendatahub-documentation project, bredamc prefers comprehensive error detection in code reviews. Continue to point out potential errors whether they are related to the code changes in the PR or not, but clearly distinguish between errors introduced in the PR versus errors in existing code.
📚 Learning: 2025-07-31T15:09:05.126Z
Learnt from: jiridanek
PR: opendatahub-io/opendatahub-documentation#888
File: modules/about-workbench-images.adoc:105-105
Timestamp: 2025-07-31T15:09:05.126Z
Learning: In the opendatahub-documentation project, ROCm workbench images come pre-integrated with additional frameworks and dependencies that users can utilize, rather than requiring users to integrate frameworks themselves. When describing these images, the focus should be on the pre-integrated capabilities rather than user integration tasks.
Applied to files:
modules/deploying-a-llamastackdistribution-instance.adoc
8d5e418
to
1d463aa
Compare
…e - modified distribution image name in example yaml and modified prereq
…ng issue with the storage not working properly
…on.name field in the docs
c8c9509
to
ef570e8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
modules/deploying-a-llamastackdistribution-instance.adoc (1)
114-114
: Using distribution.name is correct and resolves the earlier placeholder-image concernSpecifying
spec.server.distribution.name: rh-dev
(instead ofdistribution.image
) aligns with the operator-managed resolution approach discussed in this PR and addresses the “invalid image reference” feedback from earlier reviews.Please confirm that the documented Operator version in this release supports resolving the
rh-dev
alias (so readers on that version won’t see reconciliation failures). If there are version constraints, consider noting them briefly in the surrounding text.
🧹 Nitpick comments (2)
modules/deploying-a-llamastackdistribution-instance.adoc (2)
7-7
: Tighten abstract phrasing; keep key directive about distribution.nameCurrent sentence is repetitive (“You can… You can…”). Consider a concise, active rewrite.
-You can integrate LlamaStack and its retrieval-augmented generation (RAG) capabilities with your deployed Llama 3.2 model served by vLLM. You can use this integration to build intelligent applications that combine large language models (LLMs) with real-time data retrieval, providing more accurate and contextually relevant responses for your AI workloads. When you create a `LlamaStackDistribution` custom resource (CR), specify `rh-dev` in the `spec.server.distribution.name` field. +Integrate LlamaStack’s retrieval-augmented generation (RAG) with a vLLM‑served Llama 3.2 model to build applications that combine LLMs with real‑time data retrieval for more accurate, contextually relevant responses. When creating a `LlamaStackDistribution` custom resource (CR), set `spec.server.distribution.name` to `rh-dev`.
117-120
: Clarify what ‘rh-dev’ is and add guidance for pinning and disconnected clustersGood note. Minor tightening and adding two reader‑helpful points: it’s an Operator‑managed alias, pinning implications, and a reminder about mirroring for disconnected setups (you already link to mirroring guidance above).
-[NOTE] -==== -The `rh-dev` value is an internal image reference. When you create the `LlamaStackDistribution` custom resource, the {productname-short} Operator automatically resolves `rh-dev` to the container image in the appropriate registry. This internal image reference allows the underlying image to update without requiring changes to your custom resource. -==== +[NOTE] +==== +`rh-dev` is an Operator‑managed distribution alias. When you apply the `LlamaStackDistribution` custom resource, the {productname-short} Operator resolves this alias to the correct registry image. + +This indirection lets the Operator deliver image updates without modifying your custom resource. If you require a pinned version for reproducibility, follow the Operator’s guidance for selecting a fixed release/channel. + +For disconnected clusters, ensure the resolved image is mirrored to your internal registry and that image source policies are configured, as described in the mirroring documentation above. +====
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
modules/deploying-a-llama-model-with-kserve.adoc
(1 hunks)modules/deploying-a-llamastackdistribution-instance.adoc
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- modules/deploying-a-llama-model-with-kserve.adoc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
…e - modified distribution image name in example yaml and modified prereq
Description
The LlamaStackDistribution example has been modified to change the location of the distribution image. In addition, content has been removed that is now invalid. Finally, a prerequisite item has been modified so that it more realistically represents the task that should have been carried out beforehand.
How Has This Been Tested?
Merge criteria:
Summary by CodeRabbit